memory: restore Claude basic_memory capture/injection; add Codex/OMP capture + bounded reindex - #165
Merged
Merged
Conversation
… capture and bounded reindex
Reviewer's GuideThis PR restores Claude basic-memory capture and SessionStart injection when the Claude plugin is missing, adds source-aware Codex and OMP/Pi capture through shared dispatch logic, and triggers a backgrounded, watchdog-bounded, lock-protected reindex only after a digest is appended. Sequence diagram for cross-harness memory capture and reindexsequenceDiagram
participant Harness as Claude/Codex/OMP
participant Hook as session-end.sh
participant Dispatch as dispatch_basic_digest
participant Basic as basic-session-end.py
participant Vault as Memory vault
participant Index as memsearch index
Harness->>Hook: Send session payload
Hook->>Hook: classify_payload
alt Claude plugin exists
Hook->>Hook: Claude plugin session-end.sh
else Claude plugin missing, or source is Codex/OMP
Hook->>Dispatch: dispatch_basic_digest
Dispatch->>Basic: Write local digest
Basic->>Vault: Append digest
alt Digest appended
Dispatch->>Index: refresh_index_async
Index->>Index: Acquire reindex.lock
Index->>Vault: Index vault in background
end
end
Flow diagram for Claude memory fallback and context injectionflowchart TD
Start[Claude session] --> EndHook[session-end.sh]
EndHook --> Plugin{Claude plugin available?}
Plugin -->|Yes| PluginCapture[Claude plugin capture]
Plugin -->|No| BasicCapture[basic-session-end.py]
BasicCapture --> Digest[Append local session digest]
Start --> StartHook[session-start.sh]
StartHook --> PluginStart{Claude plugin available?}
PluginStart -->|Yes| PluginContext[Claude plugin context lookup]
PluginStart -->|No| BasicStart[basic-session-start.py]
BasicStart --> Context[Return additionalContext]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="memory/lib/basic_memory.py" line_range="306-312" />
<code_context>
assistant = last_assistant_text(messages)
paths = extract_paths(messages, payload)
- platform = payload.get("platform") or payload.get("agent") or payload.get("hook_event_name") or "basic"
+ platform = (
+ payload.get("platform")
+ or payload.get("agent")
+ or os.environ.get("DOTAGENTS_MEMORY_SOURCE")
+ or payload.get("hook_event_name")
+ or "basic"
+ )
model = payload.get("model")
</code_context>
<issue_to_address>
**issue (bug_risk):** `DOTAGENTS_MEMORY_SOURCE` does not reliably control the digest source label because `payload.platform` and `payload.agent` take precedence over the environment hint. A payload dispatched as Codex/OMP can therefore be written with a different source label when it also contains another platform or agent marker.
**Triggers:** When `DOTAGENTS_MEMORY_SOURCE` is set and the payload also contains a non-matching `platform` or `agent` field.
**Suggested fix:** Give `DOTAGENTS_MEMORY_SOURCE` precedence over payload platform and agent fields, or reject conflicting markers.
```suggestion
platform = (
os.environ.get("DOTAGENTS_MEMORY_SOURCE")
or payload.get("platform")
or payload.get("agent")
or payload.get("hook_event_name")
or "basic"
)
```
</issue_to_address>
### Comment 2
<location path="memory/hooks/common.sh" line_range="64-69" />
<code_context>
+ command -v memsearch >/dev/null 2>&1 || return 0
+ prepare_memory_index_env
+
+ reindex_lock="${MEMSEARCH_STATE_DIR%/}/reindex.lock"
+ # mkdir is atomic: it fails when a refresh already holds the lock, so we never
+ # spawn overlapping reindexers.
+ if ! mkdir "$reindex_lock" 2>/dev/null; then
+ return 0
+ fi
+
+ (
</code_context>
<issue_to_address>
**issue (bug_risk):** A forced termination of the background refresh leaves `reindex.lock` behind because cleanup depends on the subshell's EXIT trap. That stale directory permanently suppresses all future reindexes until it is manually removed.
**Triggers:** When the hook process or background refresh is killed with SIGKILL, or the machine terminates during an active refresh.
**Suggested fix:** Store the lock owner's PID and creation metadata and recover stale locks safely, or use a lock mechanism that is automatically removed by the operating system.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and a faulty capture or digest could persist incorrect or sensitive session content in the local memory store and inject it into later sessions; reverting the hooks would not remove digests already written. The data is local and bounded, however, so the impact can be repaired by deleting or rebuilding the affected memory and reindexing.
Blocking findings: memory/lib/basic_memory.py:312, memory/hooks/common.sh:69
… lock from a dead owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Restores automatic session-memory capture and injection for Claude Code, and adds equivalent local capture for Codex and OMP/Pi. Root cause and evidence: knowledge-audit report sections 3, 6, 8.
The registered Claude hooks delegate to a
memsearchplugins/claude-codedirectory that no longer ships inmemsearch0.2.x, and there was no fallback — so no Claude digests have been written since ~2026-08-25 and no memory context is injected at session start. Thebasic_memoryimplementation was healthy but unwired.Changes (scope:
memory/hooks/*,memory/lib/basic_memory.py,memory/tests/*)resolve_claude_memory_pluginis empty,session-end.shnow falls back to the localbasic_memorydigest andsession-start.shfalls back tobasic-session-start.py(restoringadditionalContextinjection). The plugin path stays preferred when it exists.common.sh) recognizes Codex and OMP payloads (via an explicitDOTAGENTS_MEMORY_SOURCEhint or the payload's ownagent/platformmarker) and routes them to the same localbasic_memorydigest — never the Claude plugin.stop.shalso honors this so Codex/OMP wired to their Stop event capture; plain Claude Stop stays a clean continuation so the Claude SessionEnd hook owns the full-session digest.build_digesthonorsDOTAGENTS_MEMORY_SOURCEfor thesource:label.refresh_index_asyncfires a backgroundedmemsearch indexof the vault. It never blocks the hook, is bounded by a watchdog (MEMSEARCH_REINDEX_TIMEOUT, default 120s; no dependency ontimeout(1)), and refuses to overlap a running refresh via an atomicmkdirlock.rem add -src …reminders added where the repo now documents Codex/OMP capture.Manual wiring (not installed automatically)
dotagents does not install Codex hooks or OMP extensions, so their capture is opt-in. See
memory/hooks/README-codex-omp.md.Codex — add to
~/.codex/hooks.jsonunderSessionEnd(preferred) orStop:{ "type": "command", "command": "DOTAGENTS_MEMORY_SOURCE=codex ~/.agents/memory/hooks/session-end.sh", "timeout": 30 }The
DOTAGENTS_MEMORY_SOURCE=codexenv labels the digest and makes theStop-wired path capture rather than pass through.OMP/Pi — copy the shipped extension:
It fires on
agent_endand pipes anagent: "omp"payload intosession-end.sh.Follow-up (owner of
setup_scaffold.go): the memsearch-tier setup already targets Codex for these dispatchers; addingDOTAGENTS_MEMORY_SOURCE=codexto that generated command would give Codex-labelled digests out of the box.Testing
python3 -m unittestovermemory/tests/— 27 tests pass, covering: plugin-present path unchanged (delegates, no basic digest), plugin-missing fallback (writes digest + injects context), Codex/OMP classification and source labelling, Claude Stop no-capture, and the reindex being gated on append, non-overlapping, non-blocking, and watchdog-bounded.go test ./...— passes (includes the existingsession-end.she2e).shellcheckon the touched scripts — only pre-existingCDPATH= cd(SC1007) / source (SC1091) notes; no new findings.KNOWLEDGE_DIR(real vault/hooks/settings untouched): Claude SessionEnd writes a digest, SessionStart injects it, Codex/OMP payloads produce labelled digests, and the reindex fires once.Design notes
timeout(1)dependency (absent on stock macOS) by using a background watchdog.Summary by Sourcery
Restore reliable Claude memory capture and injection while extending local digest capture to Codex and OMP/Pi with asynchronous, bounded indexing.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: